Closing AsyncClient in all tests (#871)#1219
Conversation
|
Great! At the moment you're using I'm wondering if we should instead just always be using Might be a good signal to users digging through our test cases, about the proper usage? |
|
@tomchristie httpx/tests/client/test_auth.py Lines 313 to 326 in cfda19e are long. In such cases I was adding PS moveover, these tests are not about closing. They might be provided with |
Yeah, imo, we should probably stick to |
|
Yup, fair call. I've also noticed that there seem to be a bunch of places in the test cases where we're using For example https://github.com/encode/httpx/blob/master/tests/client/test_headers.py where it looks to me like it'd be a whole bunch neater if it was just testing against a plain client. Wondering if we should do the following in order?
(Also @cdeler thanks for being so patient, there's been a whole bunch of extra stuff we've had to work through in order to approach this as carefully and throughly as this.) |
|
See #1222. |
|
I'm to work on this ticket in 2 days |
|
Sure thing - don't put yourself under pressure, or feel that you need to let us know when you will or won't be working on stuff. Thanks again for all your work on this. 😄 |
cfda19e to
a92fb80
Compare
All over the AsyncClient invocation is made using context manager or with try-finally block
a92fb80 to
ad5f651
Compare
|
I did what you asked:
|
|
Great! It looks like there's still some work we could be doing on preferring the use of I'm happy enough for this to go in whenever. Good work. 👍 |
As we discussed in #1197 (comment) , it was decided to move the test refactoring out from #1197
In this PR I added closing to all unclosed
AsyncClientin the tests